Evm should be all capitalized EVM as it is elsewhere. Alternatively, you can also just name it RuntimeAddressMapping since the EVM part is non-ambiguous. Same for the trait below.

In addition, make this trait non-implementable by having a generic impl, because we already have the AddressMapping trait.

impl<T: AsRef<[u8]>, U: pallet_evm::AddressMapping<T>> RuntimeAddressMapping for U {
  fn into_account_id_bytes(address: H160) -> Vec<u8> {
    U::into_account_id(address).as_ref().to_vec()
  }
}

You can probably also just reuse AddressMapping<Vec<u8>>, but I'm not sure if there's specialization issues.

Yeah I thought about using AddressMapping<Vec<u8>> but since there's an impl impl<T: From<H160>> already in causes ambiguous implementation since From<H160> can implemented for Vec<u8> as well. It could be solved by adding the fn into_account_id_bytes to the trait AddressMapping and may be reused while computing the account_id

impl<H: Hasher<Out = H256>> AddressMapping<AccountId32> for HashedAddressMapping<H> {
	fn into_account_id(address: H160) -> AccountId32 {
		AccountId32::from_slice(&Self::into_account_id_bytes(address));
	}

	fn into_account_id_bytes(address: H160) -> Vec<u8> {
		let mut data = [0u8; 24];
		data[0..4].copy_from_slice(b"evm:");
		data[4..24].copy_from_slice(&address[..]);
		let hash = H::hash(&data);

		Into::<[u8; 32]>::into(hash).to_owned()

	}
}

thoughts?

If you can make into_account_id_bytes a default impl, then it should work. However, this will require adding AsRef<[u8]> constraint to T, which I think is okay.

Seems in the pallet-evm Config we pass AccountId to AddressMapping which doesn't have the AsRef<[u8]> trait bound

I think this is the time we should group EGA together with M into an RPC config trait (because they can not be implied).

Could you perhaps elaborate how this trait might look like?

Basically this old PR #736

We didn't went with all the type parameters because most of them can be inferred by the compiler. In the past we only had a single one EGA which belongs to Config. The new type parameter we have in this PR also belongs to Config.

Something like this:

trait EthConfig {
   type EstimateGasAdapter;
   type RuntimeAddressMapping;
}

Then in Eth put the type parameter Config: EthConfig.

with_eth_config sounds like a constructor, which this isn't. Our previous with_estimate_gas_adapter had the same problem.

Remove this and the () impl for EthConfig?

Or alternatively, make this "no-op" set_overlay_changes. into_account_id_bytes should be able to have a default impl for AsRef<[u8]>. This may be preferred as it impact existing users who don't care about overlay changes the least.

Yeah we could go with no-op the is_enabled can stay false to signal an incoming call with balance and nonce overrides with an error that it's unsupported.

into_account_id_bytes is tightly coupled with RuntimeStorageOverride now so it should be sufficient to return the default value within the () impl.